Skip to content

fix: Subscription stream resource leak#2279

Merged
Mirko-von-Leipzig merged 4 commits into
nextfrom
sergerad-subscription-leak-fix
Jun 30, 2026
Merged

fix: Subscription stream resource leak#2279
Mirko-von-Leipzig merged 4 commits into
nextfrom
sergerad-subscription-leak-fix

Conversation

@sergerad

@sergerad sergerad commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Problem

A subscription stream client can connect with a starting block greater than the node's current tip. If that client disconnects, the resources reserved for that client are only cleaned up when the node's current tip reaches the starting block specified by the client. Clients can thereby cause resource leaks with malicious requests.

Solution

Whenever the node's current tip changes, also check for client disconnection. Release associated resources if client has disconnected at this point.

Changelog

[[entry]]
scope  = "node"
impact = "fixed"
description = "Reject subscription requests from clients which are far ahead of the local chain tip"

@Mirko-von-Leipzig

Copy link
Copy Markdown
Collaborator

Is the core problem not that we allow such requests at all?

For example, even with this fix, we still allow a client to submit with u64::MAX and they therefore reserve a stream permanently without having to do any work.

@sergerad

sergerad commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator Author

Is the core problem not that we allow such requests at all?

For example, even with this fix, we still allow a client to submit with u64::MAX and they therefore reserve a stream permanently without having to do any work.

Yea I was thinking that we should probably allow some reasonable upper limit like current tip + 100 blocks. Just in case the flexibility is beneficial for any reason in the future?

Otherwise yes, the alternative is to reject any such requests in the first place. LMK What you prefer.

@Mirko-von-Leipzig

Copy link
Copy Markdown
Collaborator

Is the core problem not that we allow such requests at all?
For example, even with this fix, we still allow a client to submit with u64::MAX and they therefore reserve a stream permanently without having to do any work.

Yea I was thinking that we should probably allow some reasonable upper limit like current tip + 100 blocks. Just in case the flexibility is beneficial for any reason in the future?

Otherwise yes, the alternative is to reject any such requests in the first place. LMK What you prefer.

What I want to avoid is logic like this PR where there are multiple "hidden" branch layers to cater for random edge case fixes.

So I think lets rather reject if its not within a small margin of the current tip e.g. like 10 blocks.

@sergerad

Copy link
Copy Markdown
Collaborator Author

Is the core problem not that we allow such requests at all?
For example, even with this fix, we still allow a client to submit with u64::MAX and they therefore reserve a stream permanently without having to do any work.

Yea I was thinking that we should probably allow some reasonable upper limit like current tip + 100 blocks. Just in case the flexibility is beneficial for any reason in the future?
Otherwise yes, the alternative is to reject any such requests in the first place. LMK What you prefer.

What I want to avoid is logic like this PR where there are multiple "hidden" branch layers to cater for random edge case fixes.

So I think lets rather reject if its not within a small margin of the current tip e.g. like 10 blocks.

I have added the 10 block reject logic. But I have also kept the early disconnect logic there too. Maybe an unnecessary optimization but still feels better to have it there than not.

Comment thread crates/store/src/state/subscription.rs

@kkovaacs kkovaacs left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

Comment thread crates/store/src/state/subscription.rs
@Mirko-von-Leipzig Mirko-von-Leipzig enabled auto-merge (squash) June 30, 2026 08:13
@Mirko-von-Leipzig Mirko-von-Leipzig merged commit 7efed71 into next Jun 30, 2026
23 checks passed
@Mirko-von-Leipzig Mirko-von-Leipzig deleted the sergerad-subscription-leak-fix branch June 30, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants